feat: @entered — ask which marked definitions one call went through - #24
Merged
Conversation
Contributor
|
📚 Docs preview: https://codes.sota-shimozono.com/ExperimentalAPI.jl/previews/PR24/ (updates on each push to this PR) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
sotashimozono
added a commit
that referenced
this pull request
Sep 7, 2026
…hat threw (#25) Two defects in `record`, both surfaced by reviewing #24 and both older than it. **A mark born while the block ran was lost — from the always-on layer too.** `record` snapshotted the probe set before calling `f` and never looked again, so a probe that came into existence during the call was entered by code that ran, counted by nobody, and left with its flag `false` for the rest of the process. `entered()` and the exit summary never learned about it either. A package extension loaded inside the block is the ordinary way this happens, and this package ships three of them. The set is now re-derived after the call, `saved` is keyed by probe rather than by position, and re-deriving uses `invokelatest` because the new probes' bindings are younger than the frame reading them. **The exception's backtrace pointed at `record`.** `throw(err)` after the `catch` manufactures a fresh backtrace, so a user debugging a failed run saw `record.jl` where `outer → mid → deep → energy` should be. Closing now happens inside the `catch` and the exception is re-raised with `rethrow()`. Both pinned by tests that fail against the unfixed file, each with a control. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
sotashimozono
added a commit
that referenced
this pull request
Sep 7, 2026
… not @time (#26) * fix: record lost a mark born during the block, and the backtrace of what threw Two defects in `record`, both found by reviewing #24 and both older than it. The first breaks the one thing the default layer promises. **A mark that came into existence WHILE the block ran was lost — from the always-on layer too.** `record` snapshotted the probe set before calling `f` and never looked again, so a probe born during the call was entered by code that ran, counted by nobody, and left with its flag `false` for the rest of the process. `entered()` and the exit summary never learned about it either, because while a recording is open the write side counts into the probe instead of setting the flag, and only `record`'s epilogue sets it — over the stale snapshot. record saw: [:tracked] entered(D) = [:tracked] …after: [:newborn, :tracked] entered(D) = [:newborn, :tracked] A package extension loaded inside the block is the ordinary way this happens, and this package ships three of them. The probe set is now re-derived after the call, `saved` is keyed by probe rather than by position, and the reconciliation runs over the union. Re-deriving needs `invokelatest`: the new probes' bindings are younger than the frame reading them, and 1.12 warns that will become an error. **The exception's backtrace pointed at `record`, not at the caller.** `throw(err)` after the `catch` block manufactures a fresh backtrace, so a user debugging a failed run saw `record.jl` and macro expansion where `outer → mid → deep → energy` should be, with nothing to say frames had been dropped — in exactly the case `record(f; rethrow = false)`'s own docstring names as the reason to use it. Closing now happens inside the `catch` and the exception is re-raised with `rethrow()`, which keeps the backtrace it arrived with. The old comment claiming this was impossible was wrong: it is impossible *after* the catch, which is where the call had drifted to. Both are pinned by tests that fail against the unfixed file, each with a control — a mark defined and never called is still absent, and the direct call is shown to carry the frames the recorded one must also carry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * wip: @entered carries the value; report and test gaps * feat: the value comes back from record, and @entered says where it is not @time The review of #24 found that `@entered` breaks the `@time` parity its own docstring claims, in two shapes with one cause: `record` takes a function, so the expression runs inside a closure. * `@entered begin x > 5 && return :early; … end` returned from the CLOSURE. `record` discarded what the closure returned, and the macro then read an unassigned `Ref` — so the computed value was silently dropped and the caller got `UndefRefError`, an error naming nothing to do with the cause. * `@entered y = f(x)` binds `y` inside the closure, so at global scope no `y` appears. The first is now fixed rather than documented: `Record` carries `value`, `record` captures what `f` returned, and the macro reads it from there. No box, nothing to leave undefined, and `record(f)` itself stops costing the caller their result — which was the only reason to hand-roll the box pattern the macro used internally. The second is inherent to a closure and is now stated next to the `@time` comparison it contradicts, with the form that does work (`y = @entered f(x)`). Also from the review, all measured against the shipped renderer rather than read: * **The docstring's sample output could not be produced by running the macro.** Hits are sorted by name, so `correlator` comes before `energy`, and the padding was one space wide on every row. Both copies — docstring and `docs/src/observing.md` — are corrected, and the sort is now stated with its reason: an order that moves with the measurement cannot be diffed. * **"seven names they did not write" was wrong.** The real captured path for `sum(inner(x) for _ in 1:n)` has thirteen, including keyword-dispatch wrappers and a generator closure. Corrected in both copies. That number was written from a simplified trace and never checked. * "1 observable marked definition **were** loaded" — the verb agreed with a different count than the noun, and `total == 1` is every package on the day it adopts this. * `@entered @somemacro …` leaked a raw `#= file:line =#` into the header, because `remove_linenums!` leaves the `LineNumberNode` that is a `:macrocall`'s second argument — and the 64-character cut then spent its budget on the file path. * `_short_expr` no longer swallows `InterruptException`. * `_report_entered` prints its header once instead of once per branch, and builds each label and count string once instead of twice. Four test gaps closed, each verified by the mutation that used to survive: deleting the whole footer, `for h in rec[1:1]`, disabling the truncation, and a `\d+` loaded-count that a hardcoded number satisfied. The footer — the line the feature exists for — had no assertion at all. 1079 assertions, green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
entered()answers this about a whole process.record(() -> f(x))answers it about one call, andis what this is built on. The macro earns its place by knowing two things a closure cannot: the
source text of the expression, and the line it was written on.
It returns the value of the expression, so it drops into existing code the way
@timedoes.The last line is the point
"Entered nothing" and "nothing is marked anywhere" are different states, and a package that has
not adopted this yet is in the second one. A report that could not tell them apart would read as
reassurance on a package where nothing had ever been declared. The two branches print different
text, and the tests assert both — a report that always printed one of them could not pass both.
What the tests pin that nothing else would
exprinto both the run andthe report doubles every count it prints, and the count is the answer. Verified by mutation:
splicing a second time makes the test read
2 == 1.@__LINE__taken on the same line, so areport naming the macro's own definition site would fail.
Two deliberate omissions, both stated in the docstring
this asks the question that needs neither.
and Base's higher-order functions are in it —
sum(f, xs)over a generator reportsdriver → sum → mapreduce → mapfoldl → mapfoldl_impl → foldl_impl → _foldl_impl → MappingRF → inner → energy, three names the reader wrote and seven they did not. Printing that is worsethan printing nothing. Separating them needs
pathsto carry which module each frame camefrom, which is a change to what
Hit.pathsmeans and belongs in its own change.public, not exported —@experimentalremains the only exported name, and a test says so.🤖 Generated with Claude Code